-
-
Notifications
You must be signed in to change notification settings - Fork 0
Testing - tern fix for analysis function refactor bugs #189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Unit Tests Summary 1 files 111 suites 3m 28s ⏱️ Results for commit 9ba1743. ♻️ This comment has been updated with latest results. |
Unit Test Performance Difference
Additional test case details
Results for commit ddac8b7 ♻️ This comment has been updated with latest results. |
The snapshot update is not related to tern - it comes from the data. I'm not sure when the data was changed, but the change to the LBT03 snapshot reflects the expected result as all pharmaverseadam::adlb |>
dplyr::filter(AVISIT == "Baseline") |>
dplyr::pull(CHG) |>
unique()
#> [1] NA Created on 2025-03-06 with reprex v2.1.1 |
this is really odd. lets follow up, @edelarua , I was wondeing when updating the snapshot, are you updating it locally on your machine? |
Yes, it happens locally and in the integration tests. The updated results make sense since the change from baseline values at the baseline visit should be 0/NA, not what we had previously. |
I would like to check the computation in tern again, perhaps we caught a bug that was not found before. The data change happened for some time already, but we have been refactoring the computation. |
Hi @shajoezhu & @Melkiades, I have pinpointed where exactly the change is coming from: https://github.com/insightsengineering/tern/pull/1401/files#diff-19af54bdeb1d75fc74da8834bca1f4d2c90d1d33c3650aa93bab0fae8505a7ceR201 The This is why the snapshot changed here and then back to the original result in this PR. I think in light of this we can merge insightsengineering/tern#1406 and then this change in. |
Thank you so much @edelarua ! this is good! |
Signed-off-by: Joe Zhu <sha.joe.zhu@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! thanks @edelarua
See insightsengineering/tern#1406